Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Adding a "Name" Column to "Flows" table #3203

Merged
merged 10 commits into from
May 30, 2024
Merged

Conversation

RODO94
Copy link
Contributor

@RODO94 RODO94 commented May 28, 2024

Add Name to Flow Table - Data Model Changes

Hasura Console

  • Added name to flows table using the Hasura Console to generate the "up" and "down" sql migrations
  • Altered the permissions on the name column inline with permissions allowed for the slug column

Up File

  • Added a comment to name column
  • Added script to populate records where name = NULL with a formatted version of the slug

Down File

  • Added DROP COLUMN

Flows.sql Script

  • Added name column

@RODO94 RODO94 requested a review from a team May 28, 2024 14:26
Copy link

github-actions bot commented May 28, 2024

🤖 Hasura Change Summary compared a subset of table metadata including permissions:

Updated Tables (1)

  • public.flows permissions:

    insert select update delete
    api / / /
    platformAdmin / / /
    public /
    teamEditor / / /
    10 added column permissions
    insert select update
    api ➕ name ➕ name ➕ name
    platformAdmin ➕ name ➕ name
    public
    teamEditor ➕ name ➕ name

@RODO94
Copy link
Contributor Author

RODO94 commented May 28, 2024

@RODO94 RODO94 changed the title Rory/add name model feat: Adding a "Name" Column to "Flows" table May 28, 2024
Copy link

github-actions bot commented May 28, 2024

Removed vultr server and associated DNS entries

comment on column "public"."feedback"."node_data" is 'The name of the flow, entered by the user and used to generate the "slug"';

UPDATE flows
SET name = slug
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be possible to use Postgres string functions to format this instead of directly transposing it.

Have a look at initcap() and replace() here - https://www.postgresql.org/docs/current/functions-string.html

Happy to talk this through if easier - just let me know 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So instead of the name = slug, I would de-slugify the slug and add it to the name?

Copy link
Contributor

@DafyddLlyr DafyddLlyr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! 🎉

All as expected but one suggestion on how we can handle the transformation of "my-flow-slug" to "My flow slug".

@RODO94 RODO94 requested a review from DafyddLlyr May 28, 2024 15:52
comment on column "public"."flows"."name" is 'The name of the flow, entered by the user and used to generate the "slug"';

UPDATE flows
SET name = replace(initcap(replace(slug,'-','xax')),'xax',' ')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I figured initcap() would just handle the first letter, not the first letter of each word. This makes it trickier for sure!

Using "xax" is clever and certainly works as we don't have this substring in any flow slugs (tested with select * from flows where slug ilike '%xax%';

However it's a little bit unintuitive and took me a while to work out what was going on here 😅 It also relies on strings which users could modify.

Is there another approach here that might be more robust? It should be possible to use substrings (e.g. upper case just first letter, concatenate with remaining lowercase string).

https://pgplayground.com/ can be handy as a quick playground for functions like this 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha, so I should be explicitly altering the first letter as it'll always output the same result no matter what the user adds (unless they use dashes), and it's clearer to read?

Something like this:

SET name = replace(concat(upper(substring(slug,1,1)),right(slug,-1)),'-',' ')

And using the above UPPER method is easier to read than my previous method, so even changing out the xax string to use something like _ wouldn't make it more robust, as shown below.

SET name = replace(initcap(replace(slug,'-','_')),'_',' ')

I've been using the Hasura SQL console to try different methods since it required less setup... any concerns with that?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No - totally legit and safe to use your local Hasura console for this also! 👌

I'd favour the first option as despite being fairly complex syntax, the intention feels pretty clear.

However, the second approach would also work well and I have no objections! 👍

As seen below, users cannot have special characters in the existing slug, so using the special character _ as a replacement string is safe -

image
image

And however unlikely it is that they use the string xax, it's certainly possible, so it's best not to use this -

image
image

@RODO94 RODO94 requested a review from DafyddLlyr May 29, 2024 13:31
Copy link
Contributor

@DafyddLlyr DafyddLlyr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor typo in the sync script, but we're good to go after that! 😄

@RODO94 RODO94 requested a review from DafyddLlyr May 29, 2024 15:14
Copy link
Contributor

@DafyddLlyr DafyddLlyr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect! 🏅

@RODO94 RODO94 merged commit 508175f into main May 30, 2024
12 checks passed
@RODO94 RODO94 deleted the rory/add-name-model branch May 30, 2024 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants